CNDB-15360 Chunk Cache inspection POC #2140
Conversation
lesnik2u
changed the title
Checklist before you submit for review
|
eolivelli
left a comment
eolivelli
left a comment
There was a problem hiding this comment.
I have left a couple of suggestions, you are on your way.
Let's add unit tests, there are some tests about ChunkCache, you can add them there.
We don't really care about the implementation of Caffeine, you will hardly have a deterministic behavior for ordering, for the unit tests it is just enough to ensure that if there is something that it is returned correctly, in any order
lesnik2u
force-pushed
the
CNDB-15360-ChunkCache
branch
from
1fa7663 to
1c9a057
Compare
lesnik2u
changed the title
| private long assignFileId(File file) | ||
| { | ||
| return nextFileId.getAndIncrement(); | ||
| long id = nextFileId.getAndIncrement(); |
| * @param limit maximum number of entries to inspect | ||
| * @param consumer consumer to process each entry | ||
| */ | ||
| public void inspectHotEntries(int limit, java.util.function.Consumer<ChunkCacheInspectionEntry> consumer) |
There was a problem hiding this comment.
why do we have 2 methods ?
can't we keep the boolean parameter ?
if you have two methods I guess that on the caller site you will have some "if (hottest) inspectHotEntries else inspectColdEntries" and you have to unroll this
| // We need to shift right to extract just the File ID portion by discarding the lower bits | ||
| int shift = CHUNK_SIZE_LOG2_BITS + READER_TYPE_BITS; | ||
|
|
||
| synchronousCache.policy().eviction().ifPresent(policy -> { |
There was a problem hiding this comment.
if there is no eviction policy we should throw and exception, because there is no concept of "hot" or "cold"
This it not going to happen, so you can simply add a precondition
lesnik2u
force-pushed
the
CNDB-15360-ChunkCache
branch
from
b236ad5 to
abf97b6
Compare
| } | ||
|
|
||
| @Test | ||
| public void testInspectEntriesWithLimit() throws IOException |
There was a problem hiding this comment.
you can put this into the parametrizedtest
public static Stream<Argument> testInspectEntriesValues() {
return ..... Arguments.of(CacheOrder.HOTTEST, 10), Arguments.of(CacheOrder.HOTTEST, 2)....
}
@ParametrizedTest
vood testInspectEntries(CacheOrder order, int limit)
There was a problem hiding this comment.
Had to put tests in separate class because in Junit4 for test's to be parameterised the entire class has to run with Parameterised.class
|
❌ Build ds-cassandra-pr-gate/PR-2140 rejected by Butler2 regressions found Found 2 new test failures
Found 2 known test failures |
What is the issue
https://github.com/riptano/cndb/issues/15360
What does this PR fix and why was it fixed
Add ability to inspect hot and cold entries inside of caffeine chunkCache